ENH: Implement snapshot saving #57
Conversation
shilorigins
force-pushed
the
devagr/save-snapshot
branch
from
5a55536
to
8d3edd4
Compare
shilorigins
force-pushed
the
devagr/save-snapshot
branch
from
011e2d9
to
b72b4f0
Compare
shilorigins
changed the title
shilorigins
force-pushed
the
devagr/save-snapshot
branch
2 times, most recently
from
e5e7fd3
to
4ff42b6
Compare
| """ | ||
| logger.debug("Gathering PVs to read") | ||
| pvs = self._gather_pvs(entry) | ||
| pvs.extend(Collection.meta_pvs) |
There was a problem hiding this comment.
I'm realizing now that we're defining meta PVs as a class variable, which prevents us from properly representing capturing changes to the meta_pv list. More concretely, if we:
- define one collection expecting a set of meta_pvs
- change the (global) list of meta_pvs
- take a snapshot with that same old collection
The new snapshot will pull the new meta_pvs, and might inadvertently be missing / duplicate PVs. This could be the behavior we want, but I think we need to find a way to make it more obvious if we do. Alternately we record meta_pvs as an instance variable (not class variable), and fill it with the global configuration on creation.
We also need to determine how we capture this in the Filestore backend, as currently this won't be recorded properly. This probably needs to be a separate dictionary / store external to the Root tree that gets stored
shilorigins
force-pushed
the
devagr/save-snapshot
branch
from
4ff42b6
to
44a04ba
Compare
The test didn't consider Readback handling because sample_database doesn't include any Readbacks. Rather than modify sample_database and possibly affect other tests, I've added a minimal fixture for considering a single Setpoint-Readback pair.
shilorigins
force-pushed
the
devagr/save-snapshot
branch
from
44a04ba
to
0fcf43b
Compare
Description
Snapshotsaving (closes Implement saving a Snapshot #32 )Client._build_snapshotandClient._gather_pvsClient._gather_datato accept any type ofEntry, and make iterative rather than recursiveMotivation and Context
Client.snapitself is simple: it gathers all of the PVs in an entry, adds in the meta PVs, asynchronously fetches data for each PV, and then builds the snapshot. This method is namedsnapbecausesaveis currently used for the method that writes entries to the backend.Since we want a function to gather all of the PVs from an entry, I decided to avoid code duplication by adding that functionality to
Client._gather_data. This extension needed some messy code to handle all of the parameter permutations, which I was able to simplify by changing the method to be iterative. I also usehasattrrather than type checks to avoid length conditionals. I includedClient._gather_pvsas a convenience method.Fetching the data is straightforward due to
ControlLayer._get_list.Building the snapshot uses a simple depth-first process.
Alternative Designs
One major alternative was to have each
Entrysubclass implement method to gather its PVs. This conflicts with the current system because the entry classes don't have access to the backend to process UUIDs.How Has This Been Tested?
New and updated tests in
test_client.pyWhere Has This Been Documented?
This PR, docstrings
Pre-merge checklist
docs/pre-release-notes.shand created a pre-release documentation page